Rename `expose_addr` to `expose_provenance` #122964

joboet · 2024-03-23T21:40:51Z

expose_addr is a bad name, an address is just a number and cannot be exposed. The operation is actually about the provenance of the pointer.

This PR thus changes the name of the method to expose_provenance without changing its return type. There is sufficient precedence for returning a useful value from an operation that does something else without the name indicating such, e.g. Option::insert and MaybeUninit::write.

Returning the address is merely convenient, not a fundamental part of the operation. This is implied by the fact that integers do not have provenance since

let addr = ptr.addr();
ptr.expose_provenance();
let new = ptr::with_exposed_provenance(addr);

must behave exactly like

let addr = ptr.expose_provenance();
let new = ptr::with_exposed_provenance(addr);

as the result of ptr.expose_provenance() and ptr.addr() is the same integer. Therefore, this PR removes the #[must_use] annotation on the function and updates the documentation to reflect the important part.

An alternative name would be expose_provenance. I'm not at all opposed to that, but it makes a stronger implication than we might want that the provenance of the pointer returned by ptr::with_exposed_provenance¹ is the same as that what was exposed, which is not yet specified as such IIUC. IMHO expose does not make that connection.

A previous version of this PR suggested expose as name, libs-api decided on expose_provenance to keep the symmetry with with_exposed_provenance.

CC @RalfJung
r? libs-api

I'm using the new name for from_exposed_addr suggested by rename ptr::from_exposed_addr -> ptr::with_exposed_provenance #122935 here. ↩

rustbot · 2024-03-23T21:41:00Z

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred to the platform-builtins intrinsics. Make sure the
LLVM backend as well as portable-simd gets adapted for the changes.

cc @antoyo, @GuillaumeGomez, @bjorn3, @calebzulawski, @programmerjake

This PR changes Stable MIR

cc @oli-obk, @celinval, @ouz-a

Portable SIMD is developed in its own repository. If possible, consider making this change to rust-lang/portable-simd instead.

cc @calebzulawski, @programmerjake

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

RalfJung · 2024-03-23T21:49:48Z

An alternative name would be expose_provenance. I'm not at all opposed to that, but it makes a stronger implication than we might want that the provenance of the pointer returned by ptr::with_exposed_provenance1 is the same as that what was exposed, which is not yet specified as such IIUC. IMHO expose does not make that connection.

I don't see how that implication would be any stronger by saying expose_provenance rather than expose.

The main benefit of expose is that it is less long. :)

rustbot · 2024-03-23T21:52:06Z

The Miri subtree was changed

cc @rust-lang/miri

RalfJung · 2024-03-23T21:57:01Z

compiler/rustc_codegen_ssa/src/mir/rvalue.rs


                let val = match *kind {
-                    mir::CastKind::PointerExposeAddress => {
+                    mir::CastKind::PointerExpose => {


FWIW in MiniRust I got rid of this "expose cast", and instead made expose something more like a NonDivergingIntrinsic. That properly reflects that it is entirely decoupled. But ofc that is hard to translate to LLVM...

Mmmh, actually... Since currently we generate ptrtoint for every conversion, including addr, we might be able to add a new intrinsic for exposing that lowers to a noop in LLVM, but can get recognized in miri, and then write expose as

intrinsics::expose(ptr); ptr.addr()

Since currently we generate ptrtoint for every conversion, including addr

Not anymore: #121282.

EDIT: Ah no that's the other direction. Still, this is not something I'd like to rely on.

But we could indeed make it an intrinsic. As a cast it is kind of an odd duck, none of our other casts has a side-effect. All other casts can be removed when their result is unused.

celinval · 2024-03-24T03:29:43Z

compiler/stable_mir/src/mir/body.rs

 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub enum CastKind {
-    PointerExposeAddress,
+    PointerExpose,


Please don't rename it here. Thanks

Then where should we put the note to rename it in the future? It's quite annoying when we clean up naming but smir still uses old (and thus more confusing / less apt) names possibly indefinitely.

Document the field instead.

If it's just a comment it will get lost. It needs to carry at least some sort of marker so that when you eventually do follow suit and fix old confusing names in smir, you can reliably find them. Something one can grep for. Like FIXME(smir-rename) or so -- I don't care about the details, but I hope the smir team can come up with a plausible process and document it.

Added a comment with FIXME(smir-rename). And just fyi, #122935 made this kind of change but didn't get caught 😄.

Amanieu · 2024-04-02T22:15:35Z

We discussed this in the libs-api meeting and we prefer expose_provenance as a method name for the symmetry with with_exposed_provenance.

bors · 2024-04-03T04:58:39Z

☔ The latest upstream changes (presumably #123396) made this pull request unmergeable. Please resolve the merge conflicts.

RalfJung · 2024-04-03T15:04:27Z

I have updated the PR description to match, feel free to tweak it further.

Amanieu · 2024-04-03T18:40:48Z

@bors r+

bors · 2024-04-03T18:40:51Z

📌 Commit 989660c has been approved by Amanieu

It is now in the queue for this repository.

joboet added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. I-libs-api-nominated Nominated for discussion during a libs-api team meeting. A-strict-provenance Area: Strict provenance for raw pointers labels Mar 23, 2024

rustbot assigned BurntSushi Mar 23, 2024

rustbot added O-hermit Operating System: Hermit O-itron Operating System: ITRON S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 23, 2024

This comment has been minimized.

Sign in to view

joboet force-pushed the pointer_expose branch from b12ab29 to f4c787d Compare March 23, 2024 21:52

RalfJung reviewed Mar 23, 2024

View reviewed changes

celinval reviewed Mar 24, 2024

View reviewed changes

joboet force-pushed the pointer_expose branch from f4c787d to 79f9039 Compare April 3, 2024 13:17

joboet changed the title ~~Rename expose_addr to expose~~ Rename expose_addr to expose_provenance Apr 3, 2024

This comment has been minimized.

Sign in to view

rename expose_addr to expose_provenance

989660c

joboet force-pushed the pointer_expose branch from 79f9039 to 989660c Compare April 3, 2024 14:00

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 3, 2024

matthiaskrgr mentioned this pull request Apr 3, 2024

Rollup of 8 pull requests #123429

Merged

bors merged commit 80d592c into rust-lang:master Apr 3, 2024

rustbot added this to the 1.79.0 milestone Apr 3, 2024

joboet deleted the pointer_expose branch April 3, 2024 22:49

dtolnay removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Sep 9, 2024

briansmith mentioned this pull request Jun 27, 2025

disallow from_exposed_addr(ptr.addr()) rust-lang/rust-clippy#10383

Open

Uh oh!

Rename expose_addr to expose_provenance #122964

Rename expose_addr to expose_provenance #122964

Uh oh!

Conversation

joboet commented Mar 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Footnotes

Uh oh!

rustbot commented Mar 23, 2024

Uh oh!

This comment has been minimized.

RalfJung commented Mar 23, 2024

Uh oh!

rustbot commented Mar 23, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung Mar 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Amanieu commented Apr 2, 2024

Uh oh!

bors commented Apr 3, 2024

Uh oh!

This comment has been minimized.

RalfJung commented Apr 3, 2024

Uh oh!

Amanieu commented Apr 3, 2024

Uh oh!

bors commented Apr 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Rename `expose_addr` to `expose_provenance` #122964

Rename `expose_addr` to `expose_provenance` #122964

joboet commented Mar 23, 2024 •

edited

Loading

RalfJung Mar 23, 2024 •

edited

Loading